I think that this also requires an update of/change in ring_parameters.py:

voltage = ring.voltage -> ring.rf_voltage

We could put the
h = round(freq*T0);
inside the track function, in
if (!Elem) { ... }
and store Harm_number in the element structure. In that case we would compute the harmonic number only once the first time we enter in the cavity and not at each turn.
We could also keep the harmonic number as optional field.

As you probably know, the core beam dynamics C code for AT, is a manual translation of the "numerical engine" for the beam dynamics Pascal library, aka Tracy-2, that I implemented 1990; having been tasked with providing an on-line model to guide the ALS commissioning. Hence, as a side effect, I could also calibrate the model; vs. a modern synchrotron light source.

Instead, for the SLS commissioning M. Böge & J. Chrin made a machine translation of it (with p2c) & implemented it as a client/server architecture:

M. Böge, J. Chrin “A Corba Based Client-Server Model for Beam Dynamics Applications at the SLS” ICALEPS 1999

Bottom line, clearly, the harmonic number is a property of the RF Cavity object/structure/class.

@swhite2401:

Just one minor comment: T0 is derived from a hard coded speed of light in at.c, this could potentially introduce minor numerical errors since we are using a different source to derive the "nominal" frequency in python. These could become visible for long term tracking.

On the principle you are right. However in the particular case of speed of light, since it's a fundamental absolute value (no uncertainty, no rounding, no error), it does not matter. The values are exactly the same (you can try and learn the 9 digits, there are no more…).

I keep in mind to look at a way to share constants.

I think that this also requires an update of/change in ring_parameters.py:

voltage = ring.voltage -> ring.rf_voltage

at/pyat/at/physics/ring_parameters.py

Line 103 in eb3fe84

voltage = ring.voltage

@jbengtsson: Thanks for pointing out, it's corrected on the master branch.

We could put the
h = round(freq*T0);
inside the track function, in
if (!Elem) { ... }
and store Harm_number in the element structure. In that case we would compute the harmonic number only once the first time we enter in the cavity and not at each turn.

Done.

@carmignani, @jbengtsson, @swhite2401: Does HarmonicNumber belongs to the cavity element ? It looks there is a debate about that. For the time being, I propose to follow @carmignani: keep it as an optional attribute. The point here is that wherever it is, it is not necessary for tracking, so we ignore it.

A more urgent question is: should this passmethod be the default one for cavities ? I'm in favour, since as @carmignani pointe out, CavityPass is wrong.

Any comment on that?

@carmignani, @jbengtsson, @swhite2401: Does HarmonicNumber belongs to the cavity element ? It looks there is a debate about that. For the time being, I propose to follow @carmignani: keep it as an optional attribute. The point here is that wherever it is, it is not necessary for tracking, so we ignore it.

In RFCavityPass the harmonic number is needed in the tracking. The proposed modification is fine, because you compute h from the Frequency and the circumference, but I would keep it as optional field. In case of very off-energy simulations, the computation could fail by one unit in the harmonic cavities.

In RFCavityPass the harmonic number is needed in the tracking. The proposed modification is fine, because you compute h from the Frequency and the circumference, but I would keep it as optional field. In case of very off-energy simulations, the computation could fail by one unit in the harmonic cavities.

No, it's not necessary ! In the formula quoted by @jbengtsson

ct -> ct - harm_no / f_rf * c

harm_no can be any integer, it does not change the result of the sine function. It's only there to avoid numerical overflow. I do not compute the harmonic number from the frequency, I compute the integer which minimises the argument of the sine function.

the harmonic number is a property of the RF cavity (not something being
computed; i.e., nonsense to me)

OK, so we can call this computed integer with another name, no problem.

What @carmignani mentions is valid for find_orbit6: there it's true, the path lengthening is computed with respect to the the nearest harmonic number, which for very large frequency deviations may be different form the real harmonic number.

the harmonic number is a property of the RF cavity (not something being
computed; i.e., nonsense to me).
In other words, it's one of the key RF engineering
parameters/considerations for a conceptual design, etc.

Possibly, but you can also consider it as a lattice property: number of buckets along the circumference. That defines the main RF system. It has much less meaning for an harmonic cavity.
Anyway, we can keep it in the cavity class, anyone is free to any attribute to an element.

The implementation proposed in this PR is perfectly fine and can be merged in its present state. This passmethod is not the default method (although I agree it should be but we may discuss this in another PR) so it has no impact on existing code.

Concerning the harmonic number, it should noted that the historical AT CavityPass does not use it. My personal feeling is that something that is not use in the trackfunction should not be requested as argument: this is only bringing confusion.

The cavity object is defined by its voltage, frequency, timelag (phase), and length as mentioned above, the harmonic number is derived from the frequency and the length of the machine: the same cavity object can be used in different lattices with difference harmonic numbers. What @lfarv proposes, to attach the harmonic number to the lattice, is therefore appropriate.

As far as I am concerned this PR represents an significant improvement and I renew my approval.

Dear All,

Nice conclusion & summary.

Bottom line, what's essential (I think) is to (strategically) make a clear
distinction between local vs. global properties of the structure/lattice;
i.e., a LEGO approach.
As for the implementation in which object to store the harmonic number:
the RF cavity or Ring; to do or not to do... I think is a matter of
convenience; depending on the use cases.
Fundamentally, as already observed/stated/concluded, it's not part of the
local beam dynamics model/integrator for the component (RF cavity).

Best regards.

No, it's not necessary ! In the formula quoted by @jbengtsson
ct -> ct - harm_no / f_rf * c
harm_no can be any integer, it does not change the result of the sine function. It's only there to avoid numerical overflow. I do not compute the harmonic number from the frequency, I compute the integer which minimises the argument of the sine function.

Yes, that is true. Then you are right and we can remove the harmonic number.